fix: use a lower file permission in file creation #18206

tac0turtle · 2023-10-23T11:06:49Z

Description

a-19

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

Summary by CodeRabbit

Security Improvement: Updated the permissions for the snapshots directory to enhance security. The new settings allow only the owner to write, while the group and others can only read.
Code Refactor: Simplified the code in the compareTS and compare functions for better readability. This change does not affect the functionality of these functions.
Documentation: Added a comment in the code to explain the permissions for the upgrade info file. This is a minor change and does not affect the software's operation.

github-actions · 2023-10-23T11:09:32Z

@tac0turtle your pull request is missing a changelog!

tac0turtle · 2023-10-23T11:11:32Z

x/upgrade/keeper/keeper.go

@@ -491,7 +491,8 @@ func (k Keeper) DumpUpgradeInfoToDisk(height int64, p types.Plan) error {
 // GetUpgradeInfoPath returns the upgrade info file path
 func (k Keeper) GetUpgradeInfoPath() (string, error) {
 	upgradeInfoFileDir := path.Join(k.getHomeDir(), "data")
-	if err := os.MkdirAll(upgradeInfoFileDir, os.ModePerm); err != nil {


@julienrbrt will anyone other than the sdk need to modify these files?

Mmh, cosmovisor needs to be able to read it and overwrite it if needed.

x/upgrade/keeper/keeper.go

server/util.go

coderabbitai · 2023-10-23T12:53:37Z

Walkthrough

This pull request primarily focuses on enhancing security through permission modifications, simplifying code for better readability, and adding explanatory comments for better understanding of the codebase.

Changes

File	Summary
`server/util.go`	Updated the permission mode from `os.ModePerm` to `0o644` for the snapshots directory, enhancing security by limiting permissions for group and others.
`store/storage/rocksdb/.../comparator.go`	Simplified function signatures in `compareTS` and `compare` by using slice shorthand notation for parameters `bz1` and `bz2`.
`x/upgrade/keeper/keeper.go`	Added a comment explaining the permissions for the upgrade info file.

🐇

In the land of code, where the shadows lie,

We tighten the locks, under the binary sky.

With a slice of shorthand, and a comment so clear,

We march towards perfection, with nothing to fear.

🎉🎉🎉

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

If you reply to a review comment from CodeRabbit, the bot will automatically respond.
To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
Note: Review comments are made on code diffs or files, not on the PR overview.
Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

coderabbitai

Review Status

Actionable comments generated: 1
Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between f2f47ff and bf89e1b.

Files selected for processing (3)

server/util.go (1 hunks)
store/storage/rocksdb/comparator.go (2 hunks)
x/upgrade/keeper/keeper.go (1 hunks)

Files skipped from review due to trivial changes (2)

store/storage/rocksdb/comparator.go
x/upgrade/keeper/keeper.go

server/util.go

odeke-em

Nice and thank you @tac0turtle! LGTM! I am an advocate of the proactive security measure of least permissions per https://cyber.orijtech.com/scsec/cosmos-hardening#file-permissions

odeke-em · 2023-10-26T04:26:38Z

@tac0turtle this branch needs some updates from main, would you like to merge it in?

(cherry picked from commit 331e106)

…18264) Co-authored-by: Marko <[email protected]>

…18263) Co-authored-by: Marko <[email protected]>

use a lower file permission

b93f392

github-actions bot added C:Store C:x/upgrade labels Oct 23, 2023

change perms on snapshot store creation

80bcd9b

tac0turtle added backport/v0.47.x PR scheduled for inclusion in the v0.47's next stable release backport/v0.50.x PR scheduled for inclusion in the v0.50's next stable release labels Oct 23, 2023

tac0turtle marked this pull request as ready for review October 23, 2023 11:09

tac0turtle requested a review from a team as a code owner October 23, 2023 11:09

github-prbot requested review from a team, facundomedica and atheeshp and removed request for a team October 23, 2023 11:09

tac0turtle commented Oct 23, 2023

View reviewed changes

tac0turtle assigned facundomedica and julienrbrt Oct 23, 2023

github-advanced-security bot found potential problems Oct 23, 2023

View reviewed changes

x/upgrade/keeper/keeper.go Fixed Show fixed Hide fixed

server/util.go Dismissed Show dismissed Hide dismissed

revert perm

bf89e1b

remove comment

f2d1060

github-actions bot removed the C:x/upgrade label Oct 23, 2023

coderabbitai bot reviewed Oct 23, 2023

View reviewed changes

server/util.go Show resolved Hide resolved

odeke-em approved these changes Oct 23, 2023

View reviewed changes

julienrbrt approved these changes Oct 24, 2023

View reviewed changes

Merge branch 'main' into marko/a-19

c3b5612

github-actions bot removed the C:Store label Oct 24, 2023

Merge branch 'main' into marko/a-19

857d0c0

tac0turtle enabled auto-merge October 26, 2023 06:13

tac0turtle added this pull request to the merge queue Oct 26, 2023

Merged via the queue into main with commit 331e106 Oct 26, 2023
55 checks passed

tac0turtle deleted the marko/a-19 branch October 26, 2023 07:50

mergify bot pushed a commit that referenced this pull request Oct 26, 2023

fix: use a lower file permission in file creation (#18206)

6f4dc20

(cherry picked from commit 331e106)

mergify bot pushed a commit that referenced this pull request Oct 26, 2023

fix: use a lower file permission in file creation (#18206)

9576c80

(cherry picked from commit 331e106)

This was referenced Oct 26, 2023

fix: use a lower file permission in file creation (backport #18206) #18263

Merged

fix: use a lower file permission in file creation (backport #18206) #18264

Merged

tac0turtle added a commit that referenced this pull request Oct 26, 2023

fix: use a lower file permission in file creation (backport #18206) (#…

d3c9653

…18264) Co-authored-by: Marko <[email protected]>

tac0turtle added a commit that referenced this pull request Oct 26, 2023

fix: use a lower file permission in file creation (backport #18206) (#…

0bc6756

…18263) Co-authored-by: Marko <[email protected]>

luchenqun mentioned this pull request Oct 29, 2023

fix(snapshot): grant execution permissions to the snapshot dir for the owner #18294

Merged

faddat mentioned this pull request Mar 20, 2024

sdk version bump rust-ninja/cosmos-sdk#1

Open

12 tasks

0xstepit mentioned this pull request Jul 1, 2024

chore(cachemulti): add Copy method evmos/cosmos-sdk#52

Merged

faddat mentioned this pull request Nov 8, 2024

faddat/many times #22476

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use a lower file permission in file creation #18206

fix: use a lower file permission in file creation #18206

tac0turtle commented Oct 23, 2023 •

edited by coderabbitai bot

Loading

github-actions bot commented Oct 23, 2023

tac0turtle Oct 23, 2023

julienrbrt Oct 23, 2023

coderabbitai bot commented Oct 23, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

coderabbitai bot left a comment

odeke-em left a comment

odeke-em commented Oct 26, 2023

fix: use a lower file permission in file creation #18206

fix: use a lower file permission in file creation #18206

Conversation

tac0turtle commented Oct 23, 2023 • edited by coderabbitai bot Loading

Description

Author Checklist

Reviewers Checklist

Summary by CodeRabbit

github-actions bot commented Oct 23, 2023

tac0turtle Oct 23, 2023

Choose a reason for hiding this comment

julienrbrt Oct 23, 2023

Choose a reason for hiding this comment

coderabbitai bot commented Oct 23, 2023 • edited Loading

Walkthrough

Changes

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

coderabbitai bot left a comment

Choose a reason for hiding this comment

odeke-em left a comment

Choose a reason for hiding this comment

odeke-em commented Oct 26, 2023

tac0turtle commented Oct 23, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 23, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)